Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update llpc to handle new version of llvm #2799

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

mbrkusanin
Copy link
Contributor

Handle upstream llvm changes for llvm/llvm-project@e4a4122 [IR] Remove zext and sext constant expressions (#71040)

@mbrkusanin mbrkusanin requested a review from a team as a code owner November 6, 2023 15:46
@amdvlk-admin

This comment was marked as outdated.

@amdvlk-admin

This comment was marked as outdated.

if (spvType->isTypeBool() || (spvType->isTypeVector() && spvType->getVectorComponentType()->isTypeBool())) {
if (constStoreValue->getType() != storeType) {
constStoreValue = ConstantFoldCastOperand(Instruction::ZExt, constStoreValue, storeType, m_m->getDataLayout());
if (!constStoreValue)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if an assert might be more appropriate here? I think the constant fold should always succeed shouldn't it unless a non-const value is passed in (in which case asserting might be the most helpful thing to do since that would be unexpected).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to an assert.

initializer = ConstantExpr::getZExt(initializer, type);
initializer = ConstantFoldCastOperand(Instruction::ZExt, initializer, type, m_m->getDataLayout());
if (!initializer)
return nullptr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - and even if returning nullptr is appropriate - it'll do that anyway since the next statement outside the if is a return of initializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to an assert.

@amdvlk-admin
Copy link

Test summary for commit 8fd9b2d

CTS tests (Failed: 0/137941)
  • Built with version 1.3.5.2
  • Ubuntu navi3x, Srdcvk
    • Passed: 35352/68939 (51.3%)
    • Failed: 0/68939 (0.0%)
    • Not Supported: 33587/68939 (48.7%)
    • Warnings: 0/68939 (0.0%)
    Ubuntu navi2x, Srdcvk
    • Passed: 35424/69002 (51.3%)
    • Failed: 0/69002 (0.0%)
    • Not Supported: 33578/69002 (48.7%)
    • Warnings: 0/69002 (0.0%)

Handle upstream llvm changes for llvm/llvm-project@e4a4122
[IR] Remove zext and sext constant expressions (#71040)
@amdvlk-admin
Copy link

Test summary for commit ca1bc71

CTS tests (Failed: 0/137941)
  • Built with version 1.3.5.2
  • Ubuntu navi3x, Srdcvk
    • Passed: 35353/68939 (51.3%)
    • Failed: 0/68939 (0.0%)
    • Not Supported: 33586/68939 (48.7%)
    • Warnings: 0/68939 (0.0%)
    Ubuntu navi2x, Srdcvk
    • Passed: 35424/69002 (51.3%)
    • Failed: 0/69002 (0.0%)
    • Not Supported: 33578/69002 (48.7%)
    • Warnings: 0/69002 (0.0%)

Copy link
Member

@dstutt dstutt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too

@mbrkusanin mbrkusanin merged commit 311635d into GPUOpen-Drivers:dev Nov 7, 2023
9 checks passed
@mbrkusanin mbrkusanin deleted the llpc-getzext-fix branch November 7, 2023 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants